Lift gretel model compatibility to separate module #30
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's here
Make it easier to find the "compatibility rules" for models by lifting the logic to its own module.
Why not add this logic to the specific model classes? Wouldn't that be more polymorphic?
The model classes (
GretelLSTM
,GretelCTGAN
, etc.) are wrappers around specific configurations from the blueprints repo. They do not represent every possible configuration of that model type. If a user wants to run a customized LSTM config, for example, they subclassGretelModel
, notGretelLSTM
:Note: they could subclass
GretelLSTM
, but 1) it's easier to tell people to just subclassGretelModel
regardless of model type, and/because 2) this ultimately treats the model configuration as the source of truth.If someone mistakenly created a custom Gretel model like this...
...Benchmark will treat this as an Amplify model, because basically all it does with the class instance is grab the config attribute (and the name—the results output will show the name as
MyGptX
.)